fix: improve onboard plugin icon handling#345
Conversation
Reviewer's GuideReplaces the custom OnboardItem widget with CommonIconButton in OnboardPlugin for consistent icon handling, removes legacy files and redundant fallback logic, and ensures proper includes and a robust icon fallback mechanism. Class diagram for OnboardPlugin refactor (OnboardItem replaced by CommonIconButton)classDiagram
class OnboardPlugin {
- bool m_pluginLoaded
- bool m_startupState
- QScopedPointer<CommonIconButton> m_onboardIcon
- QScopedPointer<Dock::TipsWidget> m_tipsLabel
- QScopedPointer<QuickPanel> m_quickPanel
+ QWidget* itemWidget(const QString &itemKey)
+ QWidget* itemTipsWidget(const QString &itemKey)
+ void displayModeChanged(const Dock::DisplayMode displayMode)
+ void loadPlugin()
}
class CommonIconButton {
+ void setIcon(const QString &icon, const QString &fallback, const QString &state)
}
OnboardPlugin --> CommonIconButton : uses
OnboardPlugin --> QuickPanel : uses
OnboardPlugin --> TipsWidget : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @mhduiy - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `plugins/dde-dock/onboard/onboardplugin.cpp:185` </location>
<code_context>
- m_onboardItem.reset(new OnboardItem);
+ m_onboardIcon.reset(new CommonIconButton);
+ m_onboardIcon->setFixedSize(Dock::DOCK_PLUGIN_ITEM_FIXED_SIZE);
+ m_onboardIcon->setIcon(":/icons/icon/keyboard-symbolic.svg");
m_proxyInter->itemAdded(this, pluginName());
</code_context>
<issue_to_address>
Hardcoded icon path may reduce theme flexibility.
Consider using a theme icon name instead of a hardcoded resource path if supporting theme customization is required.
Suggested implementation:
```cpp
m_onboardIcon.reset(new CommonIconButton);
m_onboardIcon->setFixedSize(Dock::DOCK_PLUGIN_ITEM_FIXED_SIZE);
// Use theme icon name for better theme flexibility
m_onboardIcon->setIcon("input-keyboard");
```
If `CommonIconButton::setIcon` does not support theme icon names directly, you may need to use `QIcon::fromTheme("input-keyboard", QIcon(":/icons/icon/keyboard-symbolic.svg"))` to provide a fallback. For example:
m_onboardIcon->setIcon(QIcon::fromTheme("input-keyboard", QIcon(":/icons/icon/keyboard-symbolic.svg")));
Adjust as needed based on how `CommonIconButton` expects its icon to be set.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| m_onboardItem.reset(new OnboardItem); | ||
| m_onboardIcon.reset(new CommonIconButton); | ||
| m_onboardIcon->setFixedSize(Dock::DOCK_PLUGIN_ITEM_FIXED_SIZE); | ||
| m_onboardIcon->setIcon(":/icons/icon/keyboard-symbolic.svg"); |
There was a problem hiding this comment.
suggestion: Hardcoded icon path may reduce theme flexibility.
Consider using a theme icon name instead of a hardcoded resource path if supporting theme customization is required.
Suggested implementation:
m_onboardIcon.reset(new CommonIconButton);
m_onboardIcon->setFixedSize(Dock::DOCK_PLUGIN_ITEM_FIXED_SIZE);
// Use theme icon name for better theme flexibility
m_onboardIcon->setIcon("input-keyboard");
If CommonIconButton::setIcon does not support theme icon names directly, you may need to use QIcon::fromTheme("input-keyboard", QIcon(":/icons/icon/keyboard-symbolic.svg")) to provide a fallback. For example:
m_onboardIcon->setIcon(QIcon::fromTheme("input-keyboard", QIcon(":/icons/icon/keyboard-symbolic.svg")));
Adjust as needed based on how CommonIconButton expects its icon to be set.
1. Replaced custom OnboardItem with CommonIconButton for consistent icon behavior 2. Simplified icon loading logic by removing redundant fallback checks 3. Added proper QIcon and QLogging includes 4. Removed obsolete OnboardItem class files 5. Improved icon fallback mechanism in CommonIconButton fix: 改进屏幕键盘插件图标处理 1. 使用 CommonIconButton 替代自定义的 OnboardItem 以获得一致的图标行为 2. 通过移除冗余的回退检查简化了图标加载逻辑 3. 添加了适当的 QIcon 和 QLogging 头文件包含 4. 移除了过时的 OnboardItem 类文件 5 改进了 CommonIconButton 中的图标回退机制
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, mhduiy The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
deepin pr auto review代码审查意见：
m_icon = QIcon::fromTheme(tmp).isNull() ? QIcon::fromTheme(tmpFallback) : QIcon::fromTheme(tmp);
以上是针对代码审查意见的详细说明，希望能够对你有所帮助。 |
3 participants
fix: 改进屏幕键盘插件图标处理
Summary by Sourcery
Replace custom OnboardItem with CommonIconButton and streamline icon handling.
Enhancements: